Refactor: expose resolved judging function - #194
Draft
luca-belli wants to merge 3 commits into
Draft
Conversation
Mirror of #190 for the judging side: give the judge domain an entry point that takes fully resolved values, so `vera judge` can call it without inheriting a script's argument conventions. Add `judge/run.py::run_judging` -- keyword-only, no parameter defaults. It receives resolved rubric paths, a resolved transcripts directory, and an explicit output target; loads the rubric and conversations; and calls the existing `judge_conversations`. It parses no arguments, reads no manifest, applies no defaults, chooses no output location, and does not configure debug logging. Unlike the generation side, this goes straight into the permanent `judge/` package rather than behind a temporary root-level boundary function: `judge/` is already the final home, and architecture.md has `vera judge` delegating to the judge package. Add `RubricConfig.from_paths` for construction from three resolved paths; `load` and `load_bundle` now delegate to it, so manifest reading stays out of the resolved-value path. Behavior unchanged. Reduce `judge.py::main` to CLI glue. It keeps its namespace signature -- it is this script's own entry point, not a domain one, and `run_pipeline.py` calls it unchanged -- but everything script-specific is now visibly CLI policy: model shorthand parsing, manifest resolution, output-location choice, resume validation, and `set_debug`. Two helpers isolate the policy, `_resolve_rubric_paths` and `_resolve_output_target`. Single-conversation judging stays here and is marked legacy-only, since `vera judge` drops it. Incidentally clears 14 Pyright errors in `judge.py`, all from building `judge_kwargs` as an untyped dict and unpacking it into a typed signature, which defeated checking entirely. Explicit keywords restore it. The judge runner itself is untouched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review catch: the previous commit added a third copy of "rubric bundle manifest paths are relative to the manifest's own folder" -- and put it in `judge.py`, a file scheduled for deletion. The helper itself is legacy-only, since `vera judge` resolves rubric paths through target resolution and never sees a bare manifest path. Its logic is not: `RubricConfig.load_bundle` already applied the same rule. Move it to `utils/rubric_manifest.py` as `load_manifest_rubric_paths`, beside the existing `load_manifest_personas` and `load_manifest_persona_context_template`, which resolve other manifest fields the same way. Its keys match `RubricConfig.from_paths`, so `load_bundle` collapses to one line and both paths now share one implementation that outlives `judge.py`. The third copy, in `vera_cli/targets.py`, resolves a different manifest kind and is left alone. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Loading and dispatch moved from `judge.py` into `judge.run.run_judging`, so the five `TestJudgeMain` tests patched attributes that no longer exist on the script module. Re-point them; no behavior assertion is weakened. The four folder-path tests now patch `load_conversations`, `judge_conversations`, and `RubricConfig` on `judge.run`, where they are used. Everything those tests assert about argument forwarding, output-target selection, and resume still holds end to end. `RubricConfig.load_bundle` is no longer called by `main()` at all, since the manifest is resolved to paths first, so those assertions become `RubricConfig.from_paths`. This checks more than before: instead of confirming a manifest path was forwarded, it confirms the three rubric files were actually resolved out of it, e.g. `data/SI/rubric_manifest.json` -> `data/SI/rubric.tsv` plus the two prompt files. `test_main_loads_distinct_rubric_bundles_end_to_end` keeps its real-parsing design, substituting the real `from_paths` for the real `load_bundle`, so it still proves `--rubrics` selects the bundle rather than being a no-op. The single-conversation test continues to patch `judge.py` attributes, because that path genuinely still lives there as legacy-only code. Full non-live suite: 1,023 passed, coverage 74.90%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mirror of #190 for the judging side: give the judge domain an entry point that
takes fully resolved values, so
vera judgecan call it without inheriting ascript's argument conventions.
Behavior-preserving plumbing. Adds no CLI surface —
vera judgeitself is afollow-up. Stacked on #193, which settles the design decisions this implements.
What changed
judge/run.py::run_judging(new, 125 lines) — keyword-only, no parameterdefaults. Receives resolved rubric paths, a resolved transcripts directory, and
an explicit output target; loads the rubric and conversations; calls the existing
judge_conversations. It parses no arguments, reads no manifest, applies nodefaults, picks no output location, and does not configure debug logging.
Placement differs from #190 deliberately. The generation side needed a
temporary root-level boundary (
generate.main) because the permanentgenerate/package does not exist yet.
judge/is already the final home, andarchitecture.mdhasvera judgedelegating to the judge package — so this goesstraight in, and
vera_cli/judge.pywill import it directly. One fewerindirection than the generate side carries.
RubricConfig.from_paths— construction from three resolved paths.loadand
load_bundledelegate to it, keeping manifest reading out of theresolved-value path. Behavior unchanged.
load_manifest_rubric_pathsinutils/rubric_manifest.py— resolves amanifest's three rubric files relative to the manifest, beside the existing
load_manifest_personasandload_manifest_persona_context_templatewhichresolve other fields the same way. This was review feedback: the first pass put
this logic in
judge.py, adding a third copy of the manifest-relative rule to afile scheduled for deletion, when
RubricConfig.load_bundlealready applied it.Now both share one implementation that outlives
judge.py. (vera's targetresolution needs none of it —
ResolvedTargetalready carries these paths.)judge.py::mainis now visibly CLI glue. It keeps its namespace signature —it is this script's own entry point, not a domain one, and
run_pipeline.pycalls it unchanged — but everything script-specific is now isolated as CLI
policy: model shorthand parsing, the manifest-path input form, output-location
choice, resume validation, and
set_debug._resolve_output_targetholds thethree-branch default and the resume checks. Single-conversation judging stays
here, marked legacy-only, since
vera judgedrops that mode per #193.judge/runner.pyis untouched, matching #190's "the existing large runnerdoes not grow."
Size
Smaller in substance than the line count suggests: #190 had to create the
resolved-input application function by extraction, whereas
judge_conversationsalready took resolved values — this only adds the load-and-run layer above it.
Incidental win
14 Pyright errors in
judge.pyare gone (baseline 14 → 0). All came frombuilding
judge_kwargsas an untyped dict and unpacking it into a typedsignature, which defeats checking entirely. Explicit keywords restore it. The 10
remaining errors in
judge/rubric_config.pyare pre-existing pandasSeries.__bool__issues in code this PR does not touch.Test changes
Five
TestJudgeMaintests patchedload_conversations,judge_conversations,and
RubricConfigas attributes ofjudge.py; those seams now live injudge/run.py. Re-pointed, with no behavior assertion weakened.The
RubricConfig.load_bundleassertions becamefrom_pathsand are nowstronger: instead of confirming a manifest path was forwarded, they confirm
the three rubric files were resolved out of it
(
data/SI/rubric_manifest.json→data/SI/rubric.tsvplus the two promptfiles).
test_main_loads_distinct_rubric_bundles_end_to_endkeeps itsreal-parsing design and still proves
--rubricsselects the bundle rather thanbeing a no-op.
Validation
generatebranch, so nothing regressedE501s injudge/response_models.py, untouched)judge.py,judge/run.py,utils/rubric_manifest.py: 0 errors,verified against a stashed baseline
🤖 Generated with Claude Code